Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 0.8.9 #377

Merged
merged 14 commits into from
Jun 3, 2024
Merged

Release 0.8.9 #377

merged 14 commits into from
Jun 3, 2024

Conversation

yatharthranjan
Copy link
Member

  • Added Ticwatch schemas and specifications
  • Updated Google Places type information

Copy link
Member

@this-Aditya this-Aditya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -11,11 +11,15 @@
"type": "enum",
"doc": "The predefined categories or tags assigned to different places based on their characteristics. These types can represent various categories such as restaurants, cafes, museums, parks, hotels, airports, hospitals, and more.",
"symbols": [ "ACCOUNTING", "ADMINISTRATIVE_AREA_LEVEL_1", "ADMINISTRATIVE_AREA_LEVEL_2", "ADMINISTRATIVE_AREA_LEVEL_3", "ADMINISTRATIVE_AREA_LEVEL_4", "ADMINISTRATIVE_AREA_LEVEL_5", "AIRPORT", "AMUSEMENT_PARK", "AQUARIUM", "ARCHIPELAGO", "ART_GALLERY", "ATM", "BAKERY", "BANK", "BAR", "BEAUTY_SALON", "BICYCLE_STORE", "BOOK_STORE", "BOWLING_ALLEY", "BUS_STATION", "CAFE", "CAMPGROUND", "CAR_DEALER", "CAR_RENTAL", "CAR_REPAIR", "CAR_WASH", "CASINO", "CEMETERY", "CHURCH", "CITY_HALL", "CLOTHING_STORE", "COLLOQUIAL_AREA", "CONTINENT", "CONVENIENCE_STORE", "COUNTRY", "COURTHOUSE", "DENTIST", "DEPARTMENT_STORE", "DOCTOR", "DRUGSTORE", "ELECTRICIAN", "ELECTRONICS_STORE", "EMBASSY", "ESTABLISHMENT", "FINANCE", "FIRE_STATION", "FLOOR", "FLORIST", "FOOD", "FUNERAL_HOME", "FURNITURE_STORE", "GAS_STATION", "GENERAL_CONTRACTOR", "GEOCODE", "GROCERY_OR_SUPERMARKET", "GYM", "HAIR_CARE", "HARDWARE_STORE", "HEALTH", "HINDU_TEMPLE", "HOME_GOODS_STORE", "HOSPITAL", "INSURANCE_AGENCY", "INTERSECTION", "JEWELRY_STORE", "LAUNDRY", "LAWYER", "LIBRARY", "LIGHT_RAIL_STATION", "LIQUOR_STORE", "LOCAL_GOVERNMENT_OFFICE", "LOCALITY", "LOCKSMITH", "LODGING", "MEAL_DELIVERY", "MEAL_TAKEAWAY", "MOSQUE", "MOVIE_RENTAL", "MOVIE_THEATER", "MOVING_COMPANY", "MUSEUM", "NATURAL_FEATURE", "NEIGHBORHOOD", "NIGHT_CLUB", "PAINTER", "PARK", "PARKING", "PET_STORE", "PHARMACY", "PHYSIOTHERAPIST", "PLACE_OF_WORSHIP", "PLUMBER", "PLUS_CODE", "POINT_OF_INTEREST", "POLICE", "POLITICAL", "POST_BOX", "POST_OFFICE", "POSTAL_CODE_PREFIX", "POSTAL_CODE_SUFFIX", "POSTAL_CODE", "POSTAL_TOWN", "PREMISE", "PRIMARY_SCHOOL", "REAL_ESTATE_AGENCY", "RESTAURANT", "ROOFING_CONTRACTOR", "ROOM", "ROUTE", "RV_PARK", "SCHOOL", "SECONDARY_SCHOOL", "SHOE_STORE", "SHOPPING_MALL", "SPA", "STADIUM", "STORAGE", "STORE", "STREET_ADDRESS", "STREET_NUMBER", "SUBLOCALITY_LEVEL_1", "SUBLOCALITY_LEVEL_2", "SUBLOCALITY_LEVEL_3", "SUBLOCALITY_LEVEL_4", "SUBLOCALITY_LEVEL_5", "SUBLOCALITY", "SUBPREMISE", "SUBWAY_STATION", "SUPERMARKET", "SYNAGOGUE", "TAXI_STAND", "TOURIST_ATTRACTION", "TOWN_SQUARE", "TRAIN_STATION", "TRANSIT_STATION", "TRAVEL_AGENCY", "UNIVERSITY", "VETERINARY_CARE", "ZOO"]
}], "doc": "Categorizing places based on their characteristics or attributes, this field represents the first type, if any, among the retrieved place categories.", "default": null
}], "doc": "Categorizing places based on their characteristics or attributes, This field is deprecated, as the Google Place.Type enum is deprecated. Instead, use the placeType field of string type.", "default": null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is deprecated, do we still need to keep this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, can't we just do the enum conversion on our side or is it not a fixed set of values anymore?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is deprecated, do we still need to keep this?

As per Yatharth's comment, we cannot delete existing fields in Avro schemas. Therefore, I have kept the field without removing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay I see, sorry I missed that. What about creating a new version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is for backwards compatibility and in line with the schema evolution guidelines. We can probably create a new version but not necessary IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay I see, yes, might be overkill. Thanks!

{ "name": "type2", "type": ["null", "org.radarcns.passive.google.PlacesType"], "doc": "Categorizing places based on their characteristics or attributes, This field is deprecated, as the Google Place.Type enum is deprecated. Instead, use the placeType field of string type.", "default": null },
{ "name": "type3", "type": ["null", "org.radarcns.passive.google.PlacesType"], "doc": "Categorizing places based on their characteristics or attributes, This field is deprecated, as the Google Place.Type enum is deprecated. Instead, use the placeType field of string type.", "default": null },
{ "name": "type4", "type": ["null", "org.radarcns.passive.google.PlacesType"], "doc": "Categorizing places based on their characteristics or attributes, This field is deprecated, as the Google Place.Type enum is deprecated. Instead, use the placeType field of string type.", "default": null },
{ "name": "placeType1", "type": ["null", "string"], "doc": "Categorizing places based on their characteristics or attributes, this field represents the first type, if any, among the retrieved place categories.", "default": null },
Copy link
Member

@mpgxvii mpgxvii Jun 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make the type an array of strings so we don't have to specify the place type number? Or do we not want nested types?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, this was an array, but Joris suggested not to use it due to performance overhead. This is the link for that review.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay I see. Thanks for the clarification!

Copy link
Member

@mpgxvii mpgxvii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@yatharthranjan yatharthranjan merged commit d224854 into master Jun 3, 2024
4 checks passed
@Bdegraaf1234 Bdegraaf1234 deleted the release-0.8.9 branch June 10, 2024 11:46
@Bdegraaf1234
Copy link
Member

I don't think these changes were ever merged back into dev, leading to some confusion on our end.

I merged them back now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants